Skip to content

[ML] Fix file upload with no ingest pipeline#193744

Merged
jgowdyelastic merged 5 commits intoelastic:mainfrom
jgowdyelastic:fix-file-upload-with-no-ingest-pipeline
Sep 25, 2024
Merged

[ML] Fix file upload with no ingest pipeline#193744
jgowdyelastic merged 5 commits intoelastic:mainfrom
jgowdyelastic:fix-file-upload-with-no-ingest-pipeline

Conversation

@jgowdyelastic
Copy link
Copy Markdown
Member

@jgowdyelastic jgowdyelastic commented Sep 23, 2024

With some datasets the find structure api will not generate an ingest pipeline. A recent change to how we catch and display errors during file upload means an upload with no pipeline now produces an error which aborts the upload.
Previously all pipeline creation errors were ignored and hidden from the user.

This PR changes changes the file upload endpoint to allow it to receive no ingest pipeline and also changes the UI to not display the pipeline creation step during upload.

This file can be used to test the fix.
https://github.com/elastic/eland/blob/main/tests/flights.json.gz

@jgowdyelastic
Copy link
Copy Markdown
Member Author

/ci

@jgowdyelastic jgowdyelastic marked this pull request as ready for review September 24, 2024 11:52
@jgowdyelastic jgowdyelastic requested review from a team as code owners September 24, 2024 11:52
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/ml-ui (:ml)

@kibana-ci
Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
dataVisualizer 726.5KB 726.6KB +93.0B
fileUpload 951.4KB 951.4KB +18.0B
total +111.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
fileUpload 14.8KB 14.8KB +8.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @jgowdyelastic

Copy link
Copy Markdown
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Tested with the flights data set and confirmed it is uploaded successfully with this change. Great to see the new functional test.

Copy link
Copy Markdown
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM. 👍

Copy link
Copy Markdown
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kibana-presentation changes LGTM
code review only

@jgowdyelastic jgowdyelastic merged commit ee1a147 into elastic:main Sep 25, 2024
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 25, 2024
With some datasets the find structure api will not generate an ingest
pipeline. A recent
[change](elastic#186956) to how we catch
and display errors during file upload means an upload with no pipeline
now produces an error which aborts the upload.
Previously all pipeline creation errors were ignored and hidden from the
user.

This PR changes changes the file upload endpoint to allow it to receive
no ingest pipeline and also changes the UI to not display the pipeline
creation step during upload.

This file can be used to test the fix.
https://github.com/elastic/eland/blob/main/tests/flights.json.gz

(cherry picked from commit ee1a147)
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Sep 25, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [[ML] Fix file upload with no ingest pipeline
(#193744)](#193744)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"James
Gowdy","email":"jgowdy@elastic.co"},"sourceCommit":{"committedDate":"2024-09-25T14:30:30Z","message":"[ML]
Fix file upload with no ingest pipeline (#193744)\n\nWith some datasets
the find structure api will not generate an ingest\r\npipeline. A
recent\r\n[change](#186956) to how
we catch\r\nand display errors during file upload means an upload with
no pipeline\r\nnow produces an error which aborts the
upload.\r\nPreviously all pipeline creation errors were ignored and
hidden from the\r\nuser.\r\n\r\nThis PR changes changes the file upload
endpoint to allow it to receive\r\nno ingest pipeline and also changes
the UI to not display the pipeline\r\ncreation step during
upload.\r\n\r\nThis file can be used to test the
fix.\r\nhttps://github.com/elastic/eland/blob/main/tests/flights.json.gz","sha":"ee1a147baca52dca5703663d35b66e7c44f3b676","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix",":ml","Feature:File
and Index Data Viz","Feature:File
Upload","v9.0.0","v8.16.0"],"title":"[ML] Fix file upload with no ingest
pipeline","number":193744,"url":"https://github.com/elastic/kibana/pull/193744","mergeCommit":{"message":"[ML]
Fix file upload with no ingest pipeline (#193744)\n\nWith some datasets
the find structure api will not generate an ingest\r\npipeline. A
recent\r\n[change](#186956) to how
we catch\r\nand display errors during file upload means an upload with
no pipeline\r\nnow produces an error which aborts the
upload.\r\nPreviously all pipeline creation errors were ignored and
hidden from the\r\nuser.\r\n\r\nThis PR changes changes the file upload
endpoint to allow it to receive\r\nno ingest pipeline and also changes
the UI to not display the pipeline\r\ncreation step during
upload.\r\n\r\nThis file can be used to test the
fix.\r\nhttps://github.com/elastic/eland/blob/main/tests/flights.json.gz","sha":"ee1a147baca52dca5703663d35b66e7c44f3b676"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193744","number":193744,"mergeCommit":{"message":"[ML]
Fix file upload with no ingest pipeline (#193744)\n\nWith some datasets
the find structure api will not generate an ingest\r\npipeline. A
recent\r\n[change](#186956) to how
we catch\r\nand display errors during file upload means an upload with
no pipeline\r\nnow produces an error which aborts the
upload.\r\nPreviously all pipeline creation errors were ignored and
hidden from the\r\nuser.\r\n\r\nThis PR changes changes the file upload
endpoint to allow it to receive\r\nno ingest pipeline and also changes
the UI to not display the pipeline\r\ncreation step during
upload.\r\n\r\nThis file can be used to test the
fix.\r\nhttps://github.com/elastic/eland/blob/main/tests/flights.json.gz","sha":"ee1a147baca52dca5703663d35b66e7c44f3b676"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: James Gowdy <jgowdy@elastic.co>
jgowdyelastic added a commit that referenced this pull request Sep 27, 2024
Fixes bug introduced in #193744

If a file is uploaded with an ingest pipeline, the pipeline is created,
but not used when uploading the data.

Adds tests to check that the data exists in the index and the expected
fields exist and are populated.
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 27, 2024
Fixes bug introduced in elastic#193744

If a file is uploaded with an ingest pipeline, the pipeline is created,
but not used when uploading the data.

Adds tests to check that the data exists in the index and the expected
fields exist and are populated.

(cherry picked from commit 6fd1913)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants